Add uv fallback for inline script environments (PEP 723 PR 6/16) - #1696
Conversation
|
🔒 Automated review in progress — Stella Huang (@StellaHuang95) is auto-reviewing this PR. |
2c8302b to
d58cf31
Compare
Eduardo Villalpando Mello (edvilme)
left a comment
There was a problem hiding this comment.
:)
3521a00 to
af8592c
Compare
Eleanor Boyd (eleanorjboyd)
left a comment
There was a problem hiding this comment.
Thanks for adding this fallback. I found two related gaps that need addressing:
-
In
InlineScriptEnvManager.create, fallback interpreter selection and installation happen before the per-script entry is added topendingCreations. Concurrentcreate()calls for the same script can therefore both display prompts and launch competing uv/Python installations. Please move the fallback work inside the deduplicated operation. -
The new uv-version-lookup installation path is not effectively exercised. The manager tests stub
ensureUvForInlineScriptVersionLookup()to return true, while the installer tests cover only dismissing its prompt. Please add coverage for accepted installation success, installation failure, uv remaining unavailable/restart-required, cancellation, and concurrent same-script creation.
Add consent-gated uv installation when no installed interpreter satisfies a script. Coalesce matching installs, skip prompts for quick create, and directly resolve a successful installation when discovery is stale or unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a9f6ba1-9bd3-4664-bc25-a0d34d7a2e91
af8592c to
c6abcf8
Compare
Thanks, both gaps are addressed.
|
Coalesce identical inline-script setup requests before interpreter installation and cover uv bootstrap success and failure paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a9f6ba1-9bd3-4664-bc25-a0d34d7a2e91
| private readonly pendingSetups = new Map<string, Promise<PythonEnvironment | undefined>>(); | ||
| private readonly pendingCreations = new Map<string, Promise<PythonEnvironment | undefined>>(); | ||
| private readonly directlyResolvedBaseInterpreters = new Map<string, PythonEnvironment>(); | ||
| private baseInterpreterInstallationQueue: Promise<void> = Promise.resolve(); |
There was a problem hiding this comment.
What is this used for?
f5fb6c3
into
microsoft:main
> Part of #1602 (PEP 723 inline script env support). Design doc: #1601. > > This replaces the earlier closed draft #1653 with the finalized implementation rebased on `main`. ### Roadmap context This is **PR 7 of 16** in the PEP 723 inline-script roadmap. It adds durable per-script environment associations to the internal manager. | Phase 2: Manager | PR | Status | |---|---|---| | | PR 4: `InlineScriptEnvManager` skeleton | merged (#1610) | | | PR 5a: generic env-creation utilities | merged (#1651) | | | PR 5b: inline-script cache + interpreter utilities | merged (#1655) | | | PR 5c: `create()` happy path | merged (#1656) | | | PR 6: `create()` uv-install fallback | open (#1696) | | | **PR 7: persistence (`get` / `set` + Memento)** | **this PR** | | | PR 8: activation-time discovery | follow-up | | | PR 9: route PEP 723 scripts to the inline manager | follow-up | ### Why this PR PR 5 can build or reuse an inline-script environment, but the manager does not remember that the resulting environment belongs to a particular script. After an extension-host restart, the in-memory association is gone. This PR implements the persistence portion of Q4 in the design: - maintain an independent environment association for each script; - persist script-to-environment executable paths in workspace Memento; - lazily and safely rehydrate those associations; - re-check current `requires-python` metadata before returning an environment; - report changes so the central environment API can update its last-known state. ### What this PR does **Implements per-script `set()`** - Accepts one or more local `file:` URIs and rejects invalid or mixed scopes atomically. - Validates that selected environments are owned inline-script cache entries. - Persists a normalized script path → environment executable path mapping under a dedicated Memento key. - Supports assigning and unassigning individual scripts or batches. - Updates in-memory state and emits `onDidChangeEnvironment` only for effective changes. - Leaves the existing `create()` behavior separate: creation alone does not implicitly establish a persisted association. **Implements per-script `get()`** - Reads current PEP 723 metadata before returning an association. - Keeps unreadable or temporarily invalid script metadata from destructively clearing state. - Returns an in-memory association when valid. - Lazily reconstructs persisted environments after restart instead of resolving every script during activation. - Re-checks `requires-python` against the reconstructed Python version before returning it. **Safely rehydrates persisted associations** - Requires an absolute executable path. - Preserves associations while their cache entry is locked or being created. - Verifies that the executable exists and is a regular file. - Resolves it into a `PythonEnvironment` and confirms that it belongs to the expected extension-owned cache entry. - Removes only definitively stale associations; transient filesystem or resolver failures remain retryable. - Emits a change event when a slow rehydration eventually succeeds, including when the public API's initial one-second wait has already elapsed. **Validates warm in-memory associations** - Periodically revalidates cached associations without performing full resolution on every lookup. - Detects executables deleted while VS Code remains open. - Detects an environment rebuilt at the same cache path with a different Python version. - Preserves busy/locked entries instead of misclassifying them as stale. - Coalesces simultaneous validations for the same script. - Retains the existing environment object when resolution produces only a new generated ID for the same Python, avoiding false changes and duplicate ID-keyed resources. **Protects persistence and selection from races** - Serializes Memento read-modify-write operations so concurrent script selections cannot lose one another. - Uses per-script association revisions so an older rehydration cannot overwrite a newer selection or unset. - Removes stale persisted values conditionally, only if the inspected path is still current. - Keeps failed persistence writes from changing in-memory state or emitting success-shaped events. - Does not globally serialize unrelated environment operations. **Updates central active-environment tracking** - Keys inline-script selections by normalized script path rather than containing project, so two scripts in one workspace can retain different environments. - Uses per-scope revisions and manager identity checks so slow refreshes cannot overwrite newer selections. - Ensures failed selections and failed refreshes do not discard a valid in-flight refresh. - Groups same-manager batch unsets and calls the manager once with the complete URI array. - Updates central cache entries and events only after the manager operation succeeds. - Attributes inline-script change events to the script URI rather than the containing project URI. ### Example Given two scripts in the same workspace: ```text tools/report.py → Python 3.12 inline environment tools/import.py → Python 3.13 inline environment ``` PR 7 stores and retrieves those associations independently. Selecting the environment for `import.py` does not overwrite the last-known environment for `report.py`. After restart: ```text get(report.py) → read persisted executable → verify cache ownership and current metadata → resolve environment → cache and return it ``` If `report.py` later changes from `requires-python = ">=3.11"` to `">=3.13"`, its persisted Python 3.12 environment is no longer returned as compatible. ### Persistence and failure semantics | Condition | Behavior | |---|---| | Executable exists and cache ownership is valid | Rehydrate and return | | Cache entry is locked/in progress | Preserve association; retry later | | Resolver fails transiently | Preserve association; retry later | | Executable is definitively missing and unlocked | Remove stale association and notify | | A newer selection wins during rehydration | Discard the stale result | | Memento write fails | Keep previous in-memory/persisted selection and propagate the error | ### Tests Coverage includes: - assign, retrieve, unset, and batch persistence; - restart-time lazy rehydration and delayed success events; - metadata compatibility changes; - missing, malformed, unowned, busy, and transient cache states; - warm deletion and same-path rebuild detection; - concurrent persistence, rehydration, validation, selection, and unset races; - failed Memento writes; - strict URI-scope validation; - independent same-project script selections; - stale and failed central refresh ordering; - atomic same-manager batch unsets. `npm run compile-tests`, `npm run lint`, the full unit suite, and the focused persistence/central-manager suites are clean. ### Performance - Rehydration is lazy rather than activation-blocking. - Warm associations are cached and validation is throttled. - Same-script rehydration and validation work is coalesced. - Queues cover only shared persistence and mutation ordering; unrelated script reads and environment-manager operations remain independent. ### User impact **No default-path user impact yet.** This completes an internal Phase 2 manager capability. Automatic routing and user-facing entry points arrive in later roadmap PRs. Once routing is wired, script-specific selections will survive extension-host restarts and remain independent even for multiple scripts in the same workspace. ### Merge order The core persistence behavior depends on the merged manager skeleton (#1610). This branch is rebased on current `main`; PR 8 and PR 9 build on this capability. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a9f6ba1-9bd3-4664-bc25-a0d34d7a2e91
Roadmap context
This is PR 6 of 16 in the PEP 723 inline-script roadmap. It extends the PR 5
create()happy path with the missing-compatible-interpreter fallback.InlineScriptEnvManagerskeletoncreate()happy pathcreate()uv-install fallbackget/set+ Memento)Why this PR
PR 5 can create or reuse an inline-script environment when an installed base interpreter already satisfies the script's
requires-python. It deliberately stops when no compatible interpreter exists.This PR adds the consent-gated fallback for that case:
What this PR does
Adds the inline-script fallback to
InlineScriptEnvManager.create()Selects a safe uv target from
requires-python>=3.13→3.13and==3.13.1→3.13.1.>=3.13.2,!=3.13.2without installing the excluded floor.c1→rc1) before passing a version to uv.Extends the uv installer's consent flow
Handles stale discovery after installation
requires-python, and canonicalizes its path before creating the cached environment.Examples
requires-python>=3.133.13selector==3.13.13.13.1without requiring a catalog lookup>=3.11,<3.123.11.xrelease>=3.13.2,!=3.13.23.13.2and choose a compatible advertised release>=3.15.0a1,<3.16>=3.14,<3.16Safety and concurrency
Tests
Coverage includes:
npm run compile-tests,npm run lint, the full unit suite, and the focused inline-script/uv suites are clean.User impact
No default-path user impact yet. This completes an internal Phase 2 manager capability. Automatic routing and user-facing entry points arrive in later roadmap PRs.
When those entry points are wired, users whose scripts require an unavailable Python will be able to approve installing a compatible interpreter rather than having environment creation stop.